Skip to content

ci(security): add zizmor and actionlint; refactor workflows#3473

Merged
mkoura merged 4 commits into
masterfrom
enable_zizmor
May 19, 2026
Merged

ci(security): add zizmor and actionlint; refactor workflows#3473
mkoura merged 4 commits into
masterfrom
enable_zizmor

Conversation

@mkoura
Copy link
Copy Markdown
Collaborator

@mkoura mkoura commented May 19, 2026

Address findings from zizmor GitHub Actions audit, tighten secret hygiene,
fix a step-gating bug introduced along the way, and wire actionlint into
pre-commit so the same class of bug is caught locally.

@mkoura mkoura requested a review from saratomaz as a code owner May 19, 2026 08:52
Address findings from zizmor GitHub Actions audit and tighten
secret hygiene in the reusable workflows:

- Set `persist-credentials: false` on `actions/checkout` to avoid
  leaking `GITHUB_TOKEN` via `.git/config`.
- Move `${{ inputs.* }}` interpolations out of inline shell into
  workflow-level `env:` to prevent template injection.
- Add top-level `permissions: contents: read` to all workflows that
  lacked an explicit permissions block.
- Add `cooldown.default-days: 7` to Dependabot to skip churn from
  fresh point releases.

Tighten secret exposure in the reusable workflows:

- Drop `CI_FAIL_MAILS` and `CLAUDE_CODE_OAUTH_TOKEN` from
  workflow-level `env:`; reference them via the `secrets.*` context
  in `if:` conditions instead so they are not bound to env for
  every step (including third-party actions).
- Scope `GITHUB_TOKEN` to the test runner step (which is where
  `blockers.py` actually reads it).
- Move inline `${{ secrets.TCACHE_* }}` out of `run:` blocks into
  step-level `env:` on the two curl steps.

Wire zizmor into pre-commit via zizmorcore/zizmor-pre-commit v1.25.2
and add zizmor.yml disabling the unpinned-uses rule.
@mkoura mkoura changed the title ci(security): apply zizmor audit fixes and add pre-commit hook ci(security): apply zizmor fixes and scope secrets May 19, 2026
mkoura added 2 commits May 19, 2026 11:54
GH_TOKEN was forwarded from each caller as ${{ secrets.GITHUB_TOKEN }}
and then re-exported inside the reusable workflow as GITHUB_TOKEN. The
auto-generated secrets.GITHUB_TOKEN is already available in every job,
including reusable-workflow jobs, so the alias added no permissions
and only obscured intent. Reference secrets.GITHUB_TOKEN directly in
the reusable workflows and remove the GH_TOKEN input plus the matching
line from every caller.

Fix the "Load extra env from file" step: when env-path was set but the
file did not exist, "[ -e ] && cat" returned 1 as the final command
and failed the step under bash -eo pipefail. Switch back to an if-fi
guard so a missing optional file is a no-op, not a failure.

Add a comment to zizmor.yml explaining that unpinned-uses is disabled
deliberately because workflows pin actions by major tag and rely on
Dependabot (with a 7-day cooldown) for bumps.
GHIssue.TOKEN defaults to None and the library checks "if cls.TOKEN"
before authenticating, so the explicit env-var presence check around
the assignment was redundant. Assign directly; unset or empty env
falls back to an anonymous client just as before.
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR tightens GitHub Actions security posture by applying zizmor-driven workflow hardening, reducing secret exposure, and adding zizmor to pre-commit checks.

Changes:

  • Adds explicit read-only workflow permissions and disables persisted checkout credentials.
  • Moves inline workflow inputs/secrets toward scoped env usage and removes custom GH_TOKEN plumbing.
  • Adds zizmor configuration/pre-commit integration and Dependabot cooldown for GitHub Actions updates.

Reviewed changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
zizmor.yml Configures zizmor and disables unpinned-uses.
.pre-commit-config.yaml Adds zizmor pre-commit hook.
.github/dependabot.yml Adds Dependabot cooldown for action updates.
cardano_node_tests/utils/blockers.py Simplifies GITHUB_TOKEN assignment.
.github/workflows/code_checks.yaml Adds permissions and disables checkout credential persistence.
.github/workflows/codeql.yml Disables checkout credential persistence.
.github/workflows/nix_smoke.yaml Adds permissions and disables checkout credential persistence.
.github/workflows/regression.yaml Adds permissions and removes custom GH_TOKEN secret forwarding.
.github/workflows/regression-dbsync.yaml Adds permissions and removes custom GH_TOKEN secret forwarding.
.github/workflows/regression_reusable.yaml Scopes env/secrets and checkout/token handling for regression runs.
.github/workflows/upgrade.yaml Adds permissions and removes custom GH_TOKEN secret forwarding.
.github/workflows/upgrade_reusable.yaml Scopes env/secrets and checkout/token handling for upgrade runs.
.github/workflows/nightly.yaml Adds permissions and removes custom GH_TOKEN forwarding.
.github/workflows/nightly_cli.yaml Adds permissions and removes custom GH_TOKEN forwarding.
.github/workflows/nightly_dbsync.yaml Adds permissions and removes custom GH_TOKEN forwarding.
.github/workflows/nightly_pv11.yaml Adds permissions and removes custom GH_TOKEN forwarding.
.github/workflows/nightly_upgrade.yaml Adds permissions and removes custom GH_TOKEN forwarding.
Comments suppressed due to low confidence (6)

.github/workflows/regression_reusable.yaml:137

  • The secrets context is not available in step if: conditionals, so this step will make the workflow invalid. Use an allowed context such as a non-secret step output/env flag to decide whether to run the Claude action while still passing the token only in with:.
        if: (success() || failure()) && steps.testing-step.outcome != 'success' && secrets.CLAUDE_CODE_OAUTH_TOKEN

.github/workflows/regression_reusable.yaml:148

  • This step-level if: references secrets, which GitHub Actions does not support in conditionals and will fail workflow validation. Gate this step using an allowed non-secret value, such as an output set by a prior step that checks whether the secret is present.
        if: (success() || failure()) && steps.testing-step.outcome != 'success' && secrets.CLAUDE_CODE_OAUTH_TOKEN

.github/workflows/regression_reusable.yaml:221

  • This if: condition references secrets.CI_FAIL_MAILS; secrets are not available in step conditionals, so the workflow will not validate. Use an allowed non-secret flag/output for the condition and keep the actual recipient secret only in the mail action inputs.
        if: (success() || failure()) && steps.testing-step.outcome != 'success' && secrets.CI_FAIL_MAILS && github.event_name == 'schedule'

.github/workflows/upgrade_reusable.yaml:91

  • The secrets context is not available in step if: conditionals, so this step will make the workflow invalid. Use an allowed context such as a non-secret step output/env flag to decide whether to run the Claude action while still passing the token only in with:.
        if: (success() || failure()) && steps.testing-step.outcome != 'success' && secrets.CLAUDE_CODE_OAUTH_TOKEN

.github/workflows/upgrade_reusable.yaml:102

  • This step-level if: references secrets, which GitHub Actions does not support in conditionals and will fail workflow validation. Gate this step using an allowed non-secret value, such as an output set by a prior step that checks whether the secret is present.
        if: (success() || failure()) && steps.testing-step.outcome != 'success' && secrets.CLAUDE_CODE_OAUTH_TOKEN

.github/workflows/upgrade_reusable.yaml:176

  • This if: condition references secrets.CI_FAIL_MAILS; secrets are not available in step conditionals, so the workflow will not validate. Use an allowed non-secret flag/output for the condition and keep the actual recipient secret only in the mail action inputs.
        if: (success() || failure()) && steps.testing-step.outcome != 'success' && secrets.CI_FAIL_MAILS && github.event_name == 'schedule'

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread .github/workflows/regression_reusable.yaml Outdated
Comment thread .github/workflows/upgrade_reusable.yaml Outdated
The `secrets` context is not available in step-level `if:` expressions
(only github, needs, strategy, matrix, job, runner, env, vars, steps,
inputs are). Commit a9eb015 moved CLAUDE_CODE_OAUTH_TOKEN and
CI_FAIL_MAILS out of workflow-level env to avoid leaking them into
every step, but then referenced `secrets.X` directly in step `if:`
expressions, which actionlint rejects and which GitHub silently
treats as empty — the gated steps would never run.

Hoist a non-sensitive boolean per secret to job-level env (where the
`secrets` context is allowed) and check `env.HAS_* == 'true'` in the
step ifs. The secret values themselves stay in `with:` / step-level
env only on the steps that actually consume them.

Also wire actionlint into pre-commit so this class of bug is caught
locally. `-ignore` allows the existing empty-string entries in
`workflow_dispatch` `choice` lists, which represent "use default" and
are accepted by GitHub even though actionlint flags them. Fix two
unrelated SC2001 shellcheck findings exposed by actionlint by
replacing `echo | sed 's/.../g'` with bash parameter expansion.
@mkoura mkoura changed the title ci(security): apply zizmor fixes and scope secrets ci(security): add zizmor and actionlint; refactor workflows May 19, 2026
@mkoura mkoura requested a review from Copilot May 19, 2026 10:41
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated no new comments.

@mkoura mkoura merged commit eb840ef into master May 19, 2026
8 of 9 checks passed
@mkoura mkoura deleted the enable_zizmor branch May 19, 2026 12:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants